Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added styles type, which translates human-readable values to ANSI codes #301

Merged
merged 2 commits into from
Jan 11, 2017
Merged

Added styles type, which translates human-readable values to ANSI codes #301

merged 2 commits into from
Jan 11, 2017

Conversation

muesli
Copy link
Contributor

@muesli muesli commented Jan 11, 2017

Added a translation table, so that we / users can specify human-readable color values and style settings (like "bold", "italic").

I've tried to keep it simple and not make the 'styles' struct keep a private array of style settings. That way, you can simply append to it and operate on it as you would with any other string slice. That in turn means, that we can't guarantee any values in that array being pre-translated before output. The styles.String() method compiles all set styles, translates them and returns a semicolon-concatenated string, which gets used by 'styled'.

There's an argument to be had, that pre-translating styles upon appending them would be faster if you re-use the 'styled' struct several times. I don't see this happening in code, though, so - as mentioned - I opted for simplicity.

@xiaq xiaq merged commit 421d7cc into elves:master Jan 11, 2017
@xiaq
Copy link
Member

xiaq commented Jan 11, 2017

Thanks for the PR! In the future, it would be nice if you can add unit tests against your code though.

Fine print: Personally I am sometimes too lazy to write tests, which is why I accepted your PR anyway. But it would be awesome to have tests.

@muesli
Copy link
Contributor Author

muesli commented Jan 11, 2017

Happy to still write / adapt those.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants